Skip to content

Comments

die: lowercase error messages in cat-file and related commands#2052

Open
mdferdousalam wants to merge 2 commits intogitgitgadget:masterfrom
mdferdousalam:fix-error-messages-cat-file
Open

die: lowercase error messages in cat-file and related commands#2052
mdferdousalam wants to merge 2 commits intogitgitgadget:masterfrom
mdferdousalam:fix-error-messages-cat-file

Conversation

@mdferdousalam
Copy link

@mdferdousalam mdferdousalam commented Feb 23, 2026

Lowercase die(), error() and warning() messages that begin with a
capital letter, following the CodingGuidelines.

The series is now split into two patches:

[1/2] die: lowercase "Not a valid object name" messages
Fixes the same message across all six files where it appears:
cat-file.c, describe.c, ls-tree.c, merge-base.c, read-tree.c,
and unpack-file.c, along with corresponding test expectations.

[2/2] cat-file: fix remaining error and warning message formatting
Fixes "Cannot read object" and the promisor remotes warning
in cat-file.c only.

Changes since v1:

  • Split into two patches: first fixes "Not a valid object name"
    across all files in a single patch, second fixes remaining
    cat-file.c messages separately (Junio C Hamano)
  • Performed third-party audit of projects that parse this error
    message — several widely-used projects (Gitea, Gogs, GitLab
    Gitaly, JetBrains IntelliJ, etc.) do case-sensitive string
    matching on "Not a valid object name", discussed on-list
  • Fixed Signed-off-by to use full name

cc: Junio C Hamano gitster@pobox.com
cc: Engr Md Ferdous Alam mdferdousalam1989@yahoo.com

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2026

Welcome to GitGitGadget

Hi @mdferdousalam, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that either:

  • Your Pull Request has a good description, if it consists of multiple commits, as it will be used as cover letter.
  • Your Pull Request description is empty, if it consists of a single commit, as the commit message should be descriptive enough by itself.

You can CC potential reviewers by adding a footer to the PR description with the following syntax:

CC: Revi Ewer <revi.ewer@example.com>, Ill Takalook <ill.takalook@example.net>

NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description,
because it will result in a malformed CC list on the mailing list. See
example.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "revisions:" to state which subsystem the change is about, and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form /allow. A good way to find other contributors is to locate recent pull requests where someone has been /allowed:

Both the person who commented /allow and the PR author are able to /allow you.

An alternative is the channel #git-devel on the Libera Chat IRC network:

<newcontributor> I've just created my first PR, could someone please /allow me? https://github.com/gitgitgadget/git/pull/12345
<veteran> newcontributor: it is done
<newcontributor> thanks!

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

If you want to see what email(s) would be sent for a /submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this. Note that any reviewers CC'd via the list in the PR description will not actually be sent emails.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail).

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the (raw) link), then import it into your mail program. If you use GMail, you can do this via:

curl -g --user "<EMailAddress>:<Password>" \
    --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):

Changes since v1:
- Fixed a typo in the commit message (found by ...)
- Added a code comment to ... as suggested by ...
...

To send a new iteration, just add another PR comment with the contents: /submit.

Need help?

New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join.

You may also be able to find help in real time in the developer IRC channel, #git-devel on Libera Chat. Remember that IRC does not support offline messaging, so if you send someone a private message and log out, they cannot respond to you. The scrollback of #git-devel is archived, though.

@mdferdousalam
Copy link
Author

Hi @derrickstolee, I'm a new contributor. Could you please /allow me so I can submit this patch to the mailing list? Thanks!

@mdferdousalam
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2026

Submitted as pull.2052.git.1771836302101.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2052/mdferdousalam/fix-error-messages-cat-file-v1

To fetch this version to local tag pr-2052/mdferdousalam/fix-error-messages-cat-file-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2052/mdferdousalam/fix-error-messages-cat-file-v1

@mdferdousalam
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2026

Error: 03d3eaa was already submitted

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2026

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Md Ferdous Alam via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: mdferdousalam <mdferdousalam1989@yahoo.com>
>
> The CodingGuidelines state that error messages should not begin
> with a capital letter and should not end with a full stop.  Fix
> the die(), error() and warning() messages in builtin/cat-file.c
> that violate these rules, and update the corresponding test
> expectations in t1006 and t8007.
>
> Signed-off-by: mdferdousalam <mdferdousalam1989@yahoo.com>
> ---
>     cat-file: fix error and warning message formatting
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2052%2Fmdferdousalam%2Ffix-error-messages-cat-file-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2052/mdferdousalam/fix-error-messages-cat-file-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2052

It may be cleaner to deal with "Not a valid object name %s" that
appear in 5 other .c files in addition to cat-file.c in a single
patch (touching no other messages, just the "Not a valid object
name" one), and do the rest of cat-file.c in a second patch.

Have you audited third-party software that use Git plumbing commands
like "git cat-file" to make sure that they do not expect the current
and historical spelling to make sure this change will not break them?

Other than that, looking good.  Thanks for working on it.

>
>  builtin/cat-file.c           | 8 ++++----
>  t/t1006-cat-file.sh          | 6 +++---
>  t/t8007-cat-file-textconv.sh | 2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index df8e87a81f..a8d564dd6a 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -121,7 +121,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  
>  	if (get_oid_with_context(the_repository, obj_name, get_oid_flags, &oid,
>  				 &obj_context))
> -		die("Not a valid object name %s", obj_name);
> +		die("not a valid object name %s", obj_name);
>  
>  	if (!path)
>  		path = obj_context.path;
> @@ -182,7 +182,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  	case 'p':
>  		type = odb_read_object_info(the_repository->objects, &oid, NULL);
>  		if (type < 0)
> -			die("Not a valid object name %s", obj_name);
> +			die("not a valid object name %s", obj_name);
>  
>  		/* custom pretty-print here */
>  		if (type == OBJ_TREE) {
> @@ -200,7 +200,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		buf = odb_read_object(the_repository->objects, &oid,
>  				      &type, &size);
>  		if (!buf)
> -			die("Cannot read object %s", obj_name);
> +			die("cannot read object %s", obj_name);
>  
>  		if (use_mailmap) {
>  			size_t s = size;
> @@ -910,7 +910,7 @@ static int batch_objects(struct batch_options *opt)
>  			data.skip_object_info = 1;
>  
>  		if (repo_has_promisor_remote(the_repository))
> -			warning("This repository uses promisor remotes. Some objects may not be loaded.");
> +			warning("this repository uses promisor remotes; some objects may not be loaded");
>  
>  		disable_replace_refs();
>  
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index 0eee3bb878..0283c7400d 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -705,7 +705,7 @@ do
>  		then
>  			cat >expect <<-EOF
>  			error: header for $bogus_long_oid too long, exceeds 32 bytes
> -			fatal: Not a valid object name $bogus_long_oid
> +			fatal: not a valid object name $bogus_long_oid
>  			EOF
>  		else
>  			cat >expect <<-EOF
> @@ -721,7 +721,7 @@ do
>  
>  	test_expect_success "cat-file $arg1 error on missing short OID" '
>  		cat >expect.err <<-EOF &&
> -		fatal: Not a valid object name $(test_oid deadbeef_short)
> +		fatal: not a valid object name $(test_oid deadbeef_short)
>  		EOF
>  		test_must_fail git cat-file $arg1 $(test_oid deadbeef_short) >out 2>err.actual &&
>  		test_must_be_empty out &&
> @@ -732,7 +732,7 @@ do
>  		if test "$arg1" = "-p"
>  		then
>  			cat >expect.err <<-EOF
> -			fatal: Not a valid object name $(test_oid deadbeef)
> +			fatal: not a valid object name $(test_oid deadbeef)
>  			EOF
>  		else
>  			cat >expect.err <<-\EOF
> diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
> index c3735fb50d..3a69b03794 100755
> --- a/t/t8007-cat-file-textconv.sh
> +++ b/t/t8007-cat-file-textconv.sh
> @@ -22,7 +22,7 @@ test_expect_success 'setup ' '
>  
>  test_expect_success 'usage: <bad rev>' '
>  	cat >expect <<-\EOF &&
> -	fatal: Not a valid object name HEAD2
> +	fatal: not a valid object name HEAD2
>  	EOF
>  	test_must_fail git cat-file --textconv HEAD2 2>actual &&
>  	test_cmp expect actual
>
> base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4

@mdferdousalam mdferdousalam force-pushed the fix-error-messages-cat-file branch from 03d3eaa to ceffe61 Compare February 23, 2026 17:35
The CodingGuidelines state that error messages should not begin
with a capital letter.  Lowercase the "Not a valid object name"
die() messages that appear in cat-file, describe, ls-tree,
merge-base, read-tree, and unpack-file, and update the
corresponding test expectations in t1006 and t8007.

Signed-off-by: Md Ferdous Alam <mdferdousalam1989@yahoo.com>
The CodingGuidelines state that error messages should not begin
with a capital letter and should not end with a full stop.  Fix
the remaining die() and warning() messages in builtin/cat-file.c
that violate these rules: lowercase "Cannot read object" and
reformat the promisor remotes warning to not start with a capital
letter and to use a semicolon instead of a full stop.

Signed-off-by: Md Ferdous Alam <mdferdousalam1989@yahoo.com>
@mdferdousalam mdferdousalam force-pushed the fix-error-messages-cat-file branch from ceffe61 to b785003 Compare February 23, 2026 17:40
@mdferdousalam mdferdousalam changed the title cat-file: fix error and warning message formatting die: lowercase error messages in cat-file and related commands Feb 23, 2026
@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2026

Engr Md Ferdous Alam wrote on the Git mailing list (how to reply to this email):

Junio C Hamano <gitster@pobox.com> writes:


> It may be cleaner to deal with "Not a valid object name %s" that
> appear in 5 other .c files in addition to cat-file.c in a single
> patch (touching no other messages, just the "Not a valid object
> name" one), and do the rest of cat-file.c in a second patch.


Done in v2.  The series is now split into two patches:


  [1/2] die: lowercase "Not a valid object name" messages
        (cat-file.c, describe.c, ls-tree.c, merge-base.c,
         read-tree.c, unpack-file.c and their test expectations)
  [2/2] cat-file: fix remaining error and warning message formatting
        ("Cannot read object" and the promisor remotes warning)


> Have you audited third-party software that use Git plumbing commands
> like "git cat-file" to make sure that they do not expect the current
> and historical spelling to make sure this change will not break them?


I did a broad search across GitHub.  Here is what I found:


Several widely-used projects do case-sensitive string matching on
"fatal: Not a valid object name" in stderr output from Git commands:


  - Gitea (go-gitea/gitea) -- strings.Contains() in Go
  - Gogs (gogs/gogs) -- strings.Contains() in Go
  - GitLab Gitaly -- strings.HasPrefix() in Go
  - JetBrains IntelliJ -- startsWith() in Java
  - Harness CI/CD -- strings.HasPrefix() in Go (3 locations)
  - Review Board -- startswith() in Python
  - Tencent CodeAnalysis -- regex match in Python
  - DataLad -- "in" string check in Python
  - elastic/docs tooling -- .includes() in JavaScript
  - prettier-standard -- exact === equality in JavaScript


On the other hand, these are NOT affected:


  - Pure Git reimplementations (libgit2, JGit, go-git, Dulwich,
    isomorphic-git, GitPython) generate their own messages.
  - GitKraken GitLens already uses case-insensitive matching (/i).
  - VS Code Git extension and GitHub Desktop do not match this
    specific message.
  - Many CI/CD tools (Nx, BuildKit, Skaffold, Flutter) rely on
    exit codes rather than message text.


A handful of projects already check for the lowercase form "not a
valid object name", suggesting the ecosystem is in transition, but
the majority still expect the capitalized form.


Given the risk of silently breaking error detection in projects like
Gitea, Gogs, Gitaly, and IntelliJ, I am not sure whether it is
worth proceeding with patch [1/2].  Patch [2/2] changes messages
that are far less likely to be parsed by external tools ("Cannot
read object" and the promisor remotes warning).


How would you like to proceed?  Should we:


  (a) keep both patches as-is and let downstream projects adapt,
  (b) drop patch [1/2] and only ship [2/2], or
  (c) take a different approach?


Thanks,
Md Ferdous Alam

  







On Monday, February 23, 2026 at 09:54:52 PM GMT+6, Junio C Hamano <gitster@pobox.com> wrote: 





"Md Ferdous Alam via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: mdferdousalam <mdferdousalam1989@yahoo.com>
>
> The CodingGuidelines state that error messages should not begin
> with a capital letter and should not end with a full stop.  Fix
> the die(), error() and warning() messages in builtin/cat-file.c
> that violate these rules, and update the corresponding test
> expectations in t1006 and t8007.
>
> Signed-off-by: mdferdousalam <mdferdousalam1989@yahoo.com>
> ---
>    cat-file: fix error and warning message formatting
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2052%2Fmdferdousalam%2Ffix-error-messages-cat-file-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2052/mdferdousalam/fix-error-messages-cat-file-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2052

It may be cleaner to deal with "Not a valid object name %s" that
appear in 5 other .c files in addition to cat-file.c in a single
patch (touching no other messages, just the "Not a valid object
name" one), and do the rest of cat-file.c in a second patch.

Have you audited third-party software that use Git plumbing commands
like "git cat-file" to make sure that they do not expect the current
and historical spelling to make sure this change will not break them?

Other than that, looking good.  Thanks for working on it.


>
>  builtin/cat-file.c          | 8 ++++----
>  t/t1006-cat-file.sh          | 6 +++---
>  t/t8007-cat-file-textconv.sh | 2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index df8e87a81f..a8d564dd6a 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -121,7 +121,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  
>      if (get_oid_with_context(the_repository, obj_name, get_oid_flags, &oid,
>                  &obj_context))
> -        die("Not a valid object name %s", obj_name);
> +        die("not a valid object name %s", obj_name);
>  
>      if (!path)
>          path = obj_context.path;
> @@ -182,7 +182,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>      case 'p':
>          type = odb_read_object_info(the_repository->objects, &oid, NULL);
>          if (type < 0)
> -            die("Not a valid object name %s", obj_name);
> +            die("not a valid object name %s", obj_name);
>  
>          /* custom pretty-print here */
>          if (type == OBJ_TREE) {
> @@ -200,7 +200,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>          buf = odb_read_object(the_repository->objects, &oid,
>                        &type, &size);
>          if (!buf)
> -            die("Cannot read object %s", obj_name);
> +            die("cannot read object %s", obj_name);
>  
>          if (use_mailmap) {
>              size_t s = size;
> @@ -910,7 +910,7 @@ static int batch_objects(struct batch_options *opt)
>              data.skip_object_info = 1;
>  
>          if (repo_has_promisor_remote(the_repository))
> -            warning("This repository uses promisor remotes. Some objects may not be loaded.");
> +            warning("this repository uses promisor remotes; some objects may not be loaded");
>  
>          disable_replace_refs();
>  
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index 0eee3bb878..0283c7400d 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -705,7 +705,7 @@ do
>          then
>              cat >expect <<-EOF
>              error: header for $bogus_long_oid too long, exceeds 32 bytes
> -            fatal: Not a valid object name $bogus_long_oid
> +            fatal: not a valid object name $bogus_long_oid
>              EOF
>          else
>              cat >expect <<-EOF
> @@ -721,7 +721,7 @@ do
>  
>      test_expect_success "cat-file $arg1 error on missing short OID" '
>          cat >expect.err <<-EOF &&
> -        fatal: Not a valid object name $(test_oid deadbeef_short)
> +        fatal: not a valid object name $(test_oid deadbeef_short)
>          EOF
>          test_must_fail git cat-file $arg1 $(test_oid deadbeef_short) >out 2>err.actual &&
>          test_must_be_empty out &&
> @@ -732,7 +732,7 @@ do
>          if test "$arg1" = "-p"
>          then
>              cat >expect.err <<-EOF
> -            fatal: Not a valid object name $(test_oid deadbeef)
> +            fatal: not a valid object name $(test_oid deadbeef)
>              EOF
>          else
>              cat >expect.err <<-\EOF
> diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
> index c3735fb50d..3a69b03794 100755
> --- a/t/t8007-cat-file-textconv.sh
> +++ b/t/t8007-cat-file-textconv.sh
> @@ -22,7 +22,7 @@ test_expect_success 'setup ' '
>  
>  test_expect_success 'usage: <bad rev>' '
>      cat >expect <<-\EOF &&
> -    fatal: Not a valid object name HEAD2
> +    fatal: not a valid object name HEAD2
>      EOF
>      test_must_fail git cat-file --textconv HEAD2 2>actual &&
>      test_cmp expect actual
>
> base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2026

User Engr Md Ferdous Alam <mdferdousalam1989@yahoo.com> has been added to the cc: list.

@gitgitgadget
Copy link

gitgitgadget bot commented Feb 23, 2026

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Engr Md Ferdous Alam <mdferdousalam1989@yahoo.com> writes:

> Given the risk of silently breaking error detection in projects like
> Gitea, Gogs, Gitaly, and IntelliJ, I am not sure whether it is
> worth proceeding with patch [1/2].
> ...
> How would you like to proceed?

It is prudent to treat any and all messages from plumbing commands
like cat-file as sleeping dogs and keep them undisturbed.  Even for
human end-user facing messages from Porcelain commands, we may want
to be careful, but if I recall correctly, the commands your recent
set of patches covered were all plumbing?

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant